Implement forward directive #60
Conversation
|
Apology for the rather brief document; i'm not especially familliar with svelte's internals. |
This comment has been minimized.
This comment has been minimized.
|
Have updated with thoughts from my comment above. Let me know if this should be split into a separate RFC at some point. |
|
I like this a lot! I'd definitely opt for a more separated thing, not allowing |
|
Just re-read the part on Where it might make sense is in the following situation where you want to run a function on all events: |
|
I like that it also solves forwarding |
|
I really like forwarding specific events and actions (maybe transitions and animations too?) with |
|
|
Using |
What should As for the |
|
I don't think we need any extra syntax for exposing transitions/actions. You can expose that through props alreay, which is top-down, and introducing a |
This comment has been minimized.
This comment has been minimized.
I don't think this is as clear cut as you make it out to be. For transitions I see your point and I think I agree but for actions it's a bit trickier. This makes it a lot easier to apply multiple actions, rather than having to pass in an array of them into some prop. Applying multiple actions dynamically is not very ergonomic at the moment - you have to write a utility action that takes an array of actions. Compare these two as a consumer of the component: <MyInput use:autoResize={400} use:copyOnClick={{ once: true }} />vs <MyInput actionsToApply=[{action: autoResize, props: 500}, {action: copyOnClick, props: { once: true }}]Another thing it has going for it is that immediately obvious that there is an action or transition applied to the component, I think that's worth something. |
I did discuss this slightly in the RFC itself, but i'm fairly against the idea of leaving it to the developers of component libraries to abstract away the svelte language syntax. It's painful for the library's users since there can and will be inconisistencies, and it's painful for the developers, since the method isn't especially obvious and is a whole lot of boilerplate to do something extremely simple. People know how to use the native syntax, so why not use it? |
|
Regarding actions/passing in stuff: This is true for the case where you want to expose one specific element, but fails when you want to be able to apply actions/transitions/whatever to multiple DOM elements, at which point it could get more confusing. Conceptionally, an DOM element consists of - well - one element, whereas for components it could be 0-x. |
This doesn't necessarily invalidate the use of Although custom action syntax might not seem like a lot, it ends up stacking up when you have to deal with every edge case over a large amount of components. |
that's not necessarily true, as sometimes you would want to pass an/multiple actions into the component
is that any different from forwarding multiple I really don't see the problem with having a uniform, really dynamic way of passing things down. The question is more how to do that without too much development/maintenance overhead and a simple, idiomatic API. |
|
So, i've updated the RFC: At the time of opening this PR, I was actually unaware that an element could take in more than one So i've removed the proposal for this sort of syntax: <button forward:use:internalAction={}></button>and replaced it with the equivalent <button foward:use use:internalFunction={}></button> |
|
This could be expanded to cover the use case for forwarding CSS variables, <!--Button.svelte-->
<button forward:--btn-color></button><!--Consumer.svelte-->
<Button --btn-color="#007FFF" />Unless I'm missing something, this works around the issue of css vars on components potentially messing up the css structure with the extra div, since we now know exactly which element to put |
|
@698969 I think that's also interesting, but might be better to think about later? |
|
@Tropix126 Whereas I would like it to work ONLY with named groups. Of course, it doesn't have to be a double colon, it's just a suggestion, but just naming the group makes the syntax much more unambiguous at a glance. And the fact that it provides more opportunities is just another plus. This is how I see it. |
|
I also wouldn't like the group syntax as it looks pretty alien to me. How about sticking to a similar syntax that we already have: If So with (Analog to <button on:click>
<button forward:on>
<button forward:on on:click on:focus={something}>
<button forward:on on:focus|stopPropagation>Example 1: forward click events only For example 3 one could argue, that focus should be forwarded, just as the other events, unless And of course it doesn't need to be |
|
@ptrxyz Your fourth example would still forward |
|
I just came across this today and realized it seeks to solve essentially the same problem we’re discussing in sveltejs/svelte#5218 |
Example 4 was meant to show how it would be possible to exclude events from being forwarded by adding a directive like For me, For anything else, I would probably introduce a new keyword as it's a totally different cup of tea. Let me explain my thought there: |
|
Maybe an idea on how to solve the 2nd part. Look at this code: <!-- Parent -->
<MyComponent
on:focus={willNeverHappen}
on:keyDown={handledInParent}
on:click={handledInParent}
use:actionForChild={action1_params, identifier="action1"}
use:actionForChild={action2_params, identifier="action2"}
transition:fade={animation1_params, identifier="anim1"}
transition:slide={animation2_params, identifier="anim2"}
>
<!-- this is consistent with the syntax we have now for DOM nodes: -->
<button
on:click={handledInParent}
transition:fade={animation1_params, identifier="ThisWillNeverBeUsed"
>Here, the identifier is an optional property to keep things consistent with the "normal" signature for transitions/actions but makes it possible for the child to identify them. The child component can now access the <script lang="js">
import { defaultAction } from "$actions/my_default_action'
function FunctionReturningAnAction(node) {
return $$attachables?.use?.action1 || someDefaultAction || undefined
}
</script>
<!-- Child Component -->
<button
forward:on
attach:use={FunctionReturningAnAction}
attach:transition={(this) => $$attachables.use.transition1 || undefined}
on:click
on:focus|stopPropagation
on:keyup={handleInClient}
>
<button
forward:on
attach:transition={"transition2"} // <-- should this be allowed?
on:click
on:focus|stopPropagation
on:keyup={handleInClient}
>I'd introduce a directive called
The directive expects a function that returns a matching object, so for $$attachables = {
"use": {
"action1": <callable here that represents the action `actionForChild({action1_params})`>,
"action2": <callable here that represents the action `actionForChild({action2_params})`>
},
"transition": {
"transition1" : <callable here that represents the transition `fade({animation1_params})`>,
"transition2" : <callable here that represents the transition `slide({animation2_params})`>
}
}So tl;dr:
|
@ptrxyz If this seems extraneous, I'll give another idea. Targeted SlotsDescription and example - #41 (comment) This is a suggestion for a proposal, on Declarative Actions. I think Targeted Slots solves the same problems. Although the original targets were not identical. This would solve the problems of Forward Directive and Declarative Actions all at once. My example of "named forward groups" turned into Targeted Slots: <Button>
<svelte:element slot=name1
on:click={_=>console.log("group-name1 click")}
on:mouseup={_=>console.log("group-name1 mouseup")}
>I'm a button with an action!</svelte:element>
<svelte:element slot=name2
on:click={_=>console.log("group-name2 click")}
on:mouseup={_=>console.log("group-name2 mouseup")}
>I'm a button with an action!</svelte:element>
</Button><!-- Button.svelte -->
<slot targeted:name1 this=button />
<slot targeted:name2 this=button />This would have the result: <button
on:click={_=>console.log("group-name1 click")}
on:mouseup={_=>console.log("group-name1 mouseup")}
>I'm a button with an action!</button>
<button
on:click={_=>console.log("group-name2 click")}
on:mouseup={_=>console.log("group-name2 mouseup")}
>I'm a button with an action!</button>But it's also worth seeing an example from here - #41 (comment) The point is that you can add tagName, and special and normal attributes inside the component if you want, and you can add when calling the component if you want. I also call out the author of Forward Directive @Tropix126 - please evaluate. DISCLAIMER: If one believes that "slots should be left alone", one could call it different. But it would literally work as upgraded slots. |
I think this is a conceptual misunderstanding. The direction of all directives is parent-to-child, not the other way around. The The problem that the forward directive is intended to (partially) solve is that Svelte has a bunch of syntactical magic in the form of directives that cannot be easily interacted with programatically. In a framework like React, everything is a prop & so the programmer has total control--and wrapping components is trivial. In Svelte, however, the directives are hidden from component authors, and a side effect is that non-trivial composition of components & elements becomes unreasonably difficult. The idea here should be to create something which is flexible & future-proof enough to solve forwarding for both existing and future directives, but no more complicated than necessary. I also agree with @Tropix126 that getting something is better than nothing and if that means sacrificing exclusions then so be it, though I still believe it's a problem worth taking seriously. |
|
@rgossiaux With my proposal, there is no problem with exclusions. Almost no new syntax. It's "just" a slight improvement of the slots. |
I guess it's less about liking something, more about how consistent it is within the rest of the framework. I am not aware of any other place where a About targeted slots: you suggest to remove the requirement for What I mean is, if this: <svelte:element slot="slot1"
data-something="yes"
on:click
use:action
class:bold={bold_condition}
>inner stuff</svelte:element>plus: <slot name="slot1" this="button"
data-else="no"
on:focus={function_defined_in_child}
class:bold={bold_condition}
use:action2>
<span>This is rendered when slot1 is not provided</span>
</slot>would result in this: <button
data-else="no"
on:focus={function_defined_in_child}
class:bold={bold_condition}
data-something="yes"
on:click
use:action
class:bold={bold_condition}
>inner stuff</button>If I got this right, I assume it would work. I like the idea! It's also more widely usable, not just in the context discussed here. Since we only need to transport two pieces of information then (the "what to merge" and "with what to merge this on"), one could make it a bit more crips maybe and not need the Something like this: <!-- parent -->
<MyComponent>
<svelte:element on:focus={console.log} update="native_button">inner stuff</svelte:element>
<svelte:element on:click={console.log} update="custom_component">more stuff</svelte:element>
</MyComponent><!-- child component -->
<button svelte:name="native_button">
<CustomComponent svelte:name="custom_component">would get you this: <!-- renders to this -->
<button
svelte:name='native'
on:focus={console.log}
>inner stuff</button>
<CustomComponent
svelte:name='custom'
on:click={console.log}
>more stuff</CustomComponent>
note: I was thinking if it would work to replace |
Little note: Shouldn't the name be strings? And another note: does it need to be called "targeted:name1". Can't it simply follow the syntax we already have and add a So to make some rules for this approach:
Seems a bit messy tbh. I like the shorter notation I suggested in my previous comment a bit more then. The rule set would be:
|
Ok.
Yes.
To meet the needs of Declarative Actions and Forward Directive, it is enough to work only with HTML elements.
I'm glad. :)
Here is the problem. So either In addition, the slot allows you to pass a property from child to parent, via It doesn't have to be called
This is rather out of the question. See Declarative Actions - there the most important thing is that Also consider that the normal
This is a string. According to the syntax, the absence of quotation marks is still a string. Quotation marks are only necessary if the string has whitespace characters. But this is a matter of pure HTML.
As I wrote before, sometimes you want to add a tagName in So there must be The third thing is that the Summary:
If @Tropix126 also says it's a good solution, and someone from the SvelteJS maintainers says it makes sense, then rfcs should be done. |
|
So, can you summarize how this would blend in with the topic of this RFC? I get how "beefing up" slots is a thing, but I wonder if it's also the best solution for the problems presented here. As an example, how would it look when you have a third party component library and you want to capture focus, click and keyup events from a parent component. As of now, one would have to verbosely write down all the event listeners in the child component. Could you for demonstration purposes rewrite this if your idea was implemented? <script>
// simple child component
export let caption = "Button";
</script>
<button on:click on:focus on:keyup>{caption}</button><script>
// parent component
import Button from './button.svelte'
</script>
<Button on:click={console.log} caption="Hello!" />REPL: https://svelte.dev/repl/eb2fb28276a442ff802fce2c9bc53fa1?version=3.49.0 |
|
@ptrxyz This way: <script>
// simple child component
</script>
<slot targeted:button this="button" on:click on:focus on:keyup/><script>
// parent component
import Button from './button.svelte'
</script>
<Button>
<svelte:element slot="button" on:click={console.log}>Hello!</svelte:element>
</Button>And frankly, you can consider a slot without a name when there is only one slot. Because slots allow only one slot. But I'm not sure, because then it loses the advantages of passing attributes through <script>
// simple child component
</script>
<slot targeted:default this="button" on:click on:focus on:keyup/><script>
// parent component
import Button from './button.svelte'
</script>
<Button on:click={console.log}>Hello!</Button>This is basically almost the same as Unfortunately, it requires the use of the I wonder what to do with the "default content" slots With Targeted Slot you can do it like this: <script>
// simple child component
const content = "Button";
</script>
<slot targeted:button={ {content} } this="button" on:click on:focus on:keyup/><script>
// parent component
import Button from './button.svelte'
</script>
<Button>
<svelte:element slot="button" on:click={console.log} let:content>{content}</svelte:element>
</Button>...BUT sometimes you want to put there not only text, but more HTML sub-elements. That can't be done. In theory, something like: <script>
// simple child component
</script>
<target targeted:button this="button" on:click on:focus on:keyup>Button <span>someting</span></target><script>
// parent component
import Button from './button.svelte'
</script>
<Button>
<svelte:element slot="button" on:click={console.log}></svelte:element>
</Button>...Where This could also be done in other ways, but I'll let it go for now. |
|
@Tropix126 Check out my RFCS: It solves all problems and more. I'm sure @ptrxyz will be interested. |
|
The proposal says:
I'm not very familiar with Svelte internals so maybe I'm missing something. Why can't bind directives be forwarded? |
|
|
First and foremost, For example, you can easily simulate a "bind:this" to a component's inner element by exporting a prop like so: <script>
export let someEl = null;
</script>
<div bind:this={someEl} />then access it like <Component bind:someEl={someVariable} />A similar story applies to |
|
@Tropix126 Why are you ignoring me? |
|
@lukaszpolowczyk I've been rather busy as of recent and haven't had time to look through it, although i've seen your comments. I'll have a look when I get the chance. |
|
Hello, I haven't see this before sending an similar issue. Just one remark : I think that filtering the forward type is ambigous. If a component forward to a node, all directive should be usable... |
Rendered.